Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add set_ignore_cursor_events #1884

Closed

Conversation

z4122
Copy link
Contributor

@z4122 z4122 commented Mar 16, 2021

  • Tested on all platforms changed
  • Compilation warnings were addressed
  • cargo fmt has been run on this branch
  • cargo doc builds successfully
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

On macOS and Windows, added set_ignore_cursor_events to make window catch or ignore mouse events.

Copy link
Member

@maroider maroider left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to test this later, but here's some feedback anyway.

cc @ArturKovacs, @msiglreith

src/window.rs Outdated Show resolved Hide resolved
examples/cursor_grab.rs Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@@ -219,6 +221,9 @@ impl WindowFlags {
if self.contains(WindowFlags::MAXIMIZED) {
style |= WS_MAXIMIZE;
}
if self.contains(WindowFlags::IGNORE_CURSOR_EVENT) {
style_ex |= WS_EX_TRANSPARENT | WS_EX_LAYERED;
Copy link
Member

@maroider maroider Mar 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In #1815, we decided to remove the WS_EX_LAYERED style as it is documented as being incompatible with the CS_OWNDC window class style.
When I test stuff (not this PR, mind you) on my own machine it seems to work fine, but the documentation nevertheless says it is incompatible.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.. maybe we can get rid of the CS_OWNDC part as it's mainly/only for OpenGL and I believe we can get around it. Will play around with this one a bit in the next days.


Would be nice to have reference link added here regarding hit-test behavior (https://docs.microsoft.com/de-de/windows/win32/winmsg/window-features?redirectedfrom=MSDN#layered-windows)

@ArturKovacs
Copy link
Contributor

Please elaborate on what's the benefit of adding this. To me it seems that this is identical to simply not processing mouse events coming from the event loop.

@z4122 z4122 force-pushed the feat/add_set_ignore_cursor_events branch from 12e2943 to f726617 Compare March 17, 2021 15:54
@z4122
Copy link
Contributor Author

z4122 commented Mar 17, 2021

Please elaborate on what's the benefit of adding this. To me it seems that this is identical to simply not processing mouse events coming from the event loop.

We can use this feature to create a transparent floating window uppon some other windows. Then we can draw something or just show something on this window at the same time don't effect user's normal operations. It is useful in situation like screen sharing.

@z4122 z4122 force-pushed the feat/add_set_ignore_cursor_events branch from f726617 to f83a098 Compare March 17, 2021 16:11
@maroider
Copy link
Member

maroider commented Mar 17, 2021

A concrete example of a program utilizing something like this is ScreenToGif. I couldn't see if they're using this exact technique by quickly searching through their repository.

There's actually an issue open for this already: #1434.

As noted in #1434, it isn't clear if Winit should explicitly support "overlays", but I think allowing click-through windows should be fairly inoffensive. An X11 implementation in Winit should probably be possible, although I wouldn't block this PR on it.

Also: I think this may warrant an entry in the feature matrix.

src/window.rs Outdated
///
/// - **iOS / Android / Web / X11 / Wayland:** Unsupported.
#[inline]
pub fn set_ignore_cursor_events(&self, ignore: bool) -> Result<(), ExternalError> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit of bikeshedding but I would prefer some affirmative variant of the method name as set_ignore_cursor_events(false) might be hard to read. My suggestion would be set_cursor_hittest.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://developer.apple.com/documentation/appkit/nswindow/1419354-ignoresmouseevents/
Maybe set_ignore_mouse_events(bool) is better? To keep consistent with system api.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that choosing an affirmative name is more useful here than matching the macOS naming. We should probably mention the macOS API in the documentation.

Also note that the MSDN documentation uses "hit testing" to refer to this phenomenon. https://docs.microsoft.com/en-us/windows/win32/winmsg/window-features#layered-windows

src/window.rs Outdated Show resolved Hide resolved
examples/cursor_grab.rs Outdated Show resolved Hide resolved
src/platform_impl/macos/util/async.rs Outdated Show resolved Hide resolved
@z4122 z4122 force-pushed the feat/add_set_ignore_cursor_events branch from f83a098 to a5ab8e6 Compare May 11, 2021 15:43
@coderedart
Copy link

Just wanted to mention that Qt uses the Qt::WindowTransparentForInput as a widget attribute and I believe it is a much more intuitive name. Or something like input passthrough.

@ArturKovacs
Copy link
Contributor

We are starting to go deep into bikeshedding territory; it's a scary place where I don't want to be. @z4122 please rename set_ignore_mouse_events to set_cursor_hittest, and also rename WindowFlags::IGNORE_CURSOR_EVENT to CURSOR_HITTEST, and everything else that's related and I missed.

(WindowTransparentForInput in Qt seems to ignore both mouse and keyboard events but this PR only ignores mouse events)

@maroider
Copy link
Member

maroider commented Jun 21, 2021

Coming back to this PR after about a month is a bit disorienting, but I believe we're currently blocked on:

@kmod-midori
Copy link

Is there some way I could help to get this feature merged? I am targeting Windows in particular, although a cross-platform solution is also desired (don't know if we'll ever able to implement this on Linux).

@kchibisov
Copy link
Member

Closing in favor of #2232.

@kchibisov kchibisov closed this Apr 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

7 participants